SW uses oauth.js, not auth.js, to get the access token, which is why you're getting nothing back from auth.checkBearerToken(). Auth is the module that PFE uses to log in and out, reset passwords and register new volunteers with Panoptes.

I thought I could just import the Auth module, but obviously that doesn't work. @eatyourgreens Am I right in thinking that there are no methods in OAuth currently that check sessions/refresh it if needed that don't involve an iframe (which isn't used anymore, since your latest changes to the client)?

Nothing in my changes touched the iframe code, so it's still in use for refreshing tokens, if you allow it in your browser settings.

One solution would be to mirror the call that the client makes before every request with the Auth module.
https://github.com/zooniverse/panoptes-javascript-client/blob/965580bba1ed57280482a11e3a3ff30b2c9cce2d/lib/api-client.js#L10-L13

apiClient.beforeEveryRequest = function() {
    return oauth.checkBearerToken();
  },

but you're right, the OAuth module doesn't have a checkBearerToken method.

@eatyourgreens wouldn't that solution still rely on allowing iframes in the browser? Or do you mean it as "we need to write a method similar to checkBearerToke in OAuth, which doesn't rely on iframes"?

@simoneduca zooniverse/panoptes-javascript-client#76 describes authorisation in more detail.

Atm OAuth _refreshBearerToken relies on creating an iframe to check the token. I suppose I could just check sessionStorage, since that's available now. Would that make sense?

Thanks, re-reading than now.

oauth.listen(callback) might be the easiest way to be notified when oauth changes. Roger’s documentation for the API client has a description of the listen method, I think.

Just realised there’s a bug in SW’s use of storage too, in that the version number is part of the storage key so that volunteers couldn’t access their stored data if the version number ever changed. Also, the old data would never be deleted. This is a completely separate issue, however.

Cool, nice catch. How did you see that? Decoding the token?

OAuth extends Model, which is an event emitter. There’s a little bit about it in the Readme for json-api-client too.

The key bug? By looking at the names of the storage keys in the dev console.

Yes. Which key? I can only see the keys access_token, expires_in and token-type.

Got it, just saw the issue.

I was thinking of checking the token validity every time the user submits a classification. I think this would work fine for most projects, but I know some SW users can be on a single subject for more than two hours before they submit a classification. In that situation I think a warning to save their work and login before submitting should be enough.
Alternatively, listening on any changes in the token object could work, but I'm not sure where is the best place in the app to set app such listener.

How about my suggestion earlier in this thread, to run the check before each request? #369 (comment)

That's what I'd love to do and I intended my thought of checking the token before submitting classifications in that vein. But obviously I'm missing something. I don't know how to call it on every request. Do you mean to have a OAuth.checkBearerToken() for login, one for loading the app, for submitting the classification, etc.?

Doesn’t the code in my comment work?

Probably, but my question is about where to put it.

SW is your app, so that’s up to you, but it should be added wherever the API client is first configured.

I've staged the latest changes https://preview.zooniverse.org/shakespearesworld/#!/ Angular is complaining about a circular dependency's been introduced. Looking at that.

Restaged https://preview.zooniverse.org/shakespearesworld/#!/

With the latest changes, if I uncomment the alert(), I'm still seeing the alert twice if I open the transcription interface while logged in, but with an expired token. Is the request to load subjects still firing after the request to load subject sets fails? I don't know how to debug an Angular app, so I'm lost trying to see what's going on, short of watching for API requests in the network panel of the dev tools.

Is the request to load subjects still firing after the request to load subject sets fails?

It is and I don't know how to stop it. I don't think the problem is angular-specific.

That's what my pseudo-code example was supposed to be showing: that subjects and aggregations could be chained to the initial subject set loading, because it doesn't make sense to load subjects if subject sets aren't available. Failure anywhere would then abort the entire chain of requests.

Have you talked to @shaunanoordin about how he's handling expired sessions for ASM? zooniverse/anti-slavery-manuscripts#251 is pretty much ready to merge, and it's basically this PR but for ASM. Can that code be ported across to SW, or are the two apps too different for that?

are the two apps too different for that?

I haven't talked to him but I had a several looks at the code and I think they are. For one thing, I can't see any other actions being chained after the promise is rejected.

With the latest changes, I can still see the alert twice and two errors:

OK, alert showing only once now. One error thrown caught at the end by the handler, as expected. The only thing left to fix, I think, is the transcribe page still showing. I need to cancel the route state transition to do that.

Why do you need to cancel the transcribe page? I’m not following what the problem is there, if API errors are being handled correctly.

Re. #369 (comment) your code doesn’t chain that function either, so you are essentially writing the same function that has already been written for ASM.

Showing the transcribe page even in the background didn't seem very elegant. Now the error should be handled correctly. The alert shows once and on OK you're redirected, without the transcribe page being shown.

I think this work as expected now. I went around in circles for a while trying to checking for ui-router transitions as well. But in the end, the bug was just how errors were handled in the promise chain. Thanks for the help, Jim.

The use case where session expires while users are transcribing and click I'm done seems still a bit odd. The classification options pop up

(Need to remove "View your work or", as it doesn't make sense since pdf functionality was removed.)
Then the session expired alert shows twice or thrice.

Note: reproduce this you'll have to change the staging appId to match the production one and use env=production in the url.

Is that the classification save failing, but other API requests still being made anyway? Each request then firing off an alert box.

ASM have launched their fix for classification save failures today, where they open a dialog box to tell you that there was a problem, save any unfinished work to local storage then refresh to log you back in.

@eatyourgreens would you mind having one more playing around with this please? I have looked at the AMS one more time and I'll chat with Shaun later, but I'd like to hear if you see any problems with how's the app behaving now. I could improve UI by using a modal rather than alert(), but apart from that I don't see what else I should change. Errors are handled correctly and login/redirects work with disabled 3rd party cookies. Cheers.

@simoneduca did you manage to fix the bug you noted in this comment? #369 (comment)

No that still happens, but I wouldn't call it a bug. Poor UX rather: the classifications-options modal shows up because clicking I'm done doesn't fire a request. That happens once an option is chosen and then the login redirect is triggered.

Is it something that can be fixed? You said the alert was opening two or three times.

Anyway, I can come back to this when I'm finished with the feedback PR, but that might not be this week.

If my token expires while I'm transcribing, the alert box pops up when I try to submit my classification but I'm not logged back in after pressing OK. I'm also seeing the alert box pop up more than once.

From the console errors, sign-in is trying to call this URL, which doesn't work. https://panoptes.zooniverse.org/users/sign_in/?now=1521113774236. I'm not sure why that's happening.

EDIT: nevermind. localhost isn't allowed to authenticate to Panoptes production, hence the error.

That's correct. I'm trying to solve this problem

If my token expires while I'm transcribing, the alert box pops up when I try to submit my classification but I'm not logged back in after pressing OK. I'm also seeing the alert box pop up more than once.

Roger pointed out a way of solving the multiple alerts issue is to wait until all promises are resolved on submitting a classification before doing anything else.

@rogerhutchings using Promise.all(allPromises) doesn't seem to help. The app just gets stuck in a different kind a loop, as you noticed, where it just tries to log in and hence firing an alert.

function _submitToApi(newClassification) {

        var rejectedClassifications = [];

        localStorageService.set('classificationsToSubmit', localStorageService.get('classificationsToSubmit').concat([newClassification]));

        var classifications = localStorageService.get('classificationsToSubmit');

        var promises = classifications.map(function(classification) {
            var newResource = zooAPI.type('classifications')
                .create(classification);
            return newResource.save()
                .then(function (response) {
                    console.log('Classification saved', response.id);
                    return response;
                })
                .catch(function (error) {
                    console.log('Error submitting classification:', error);
                    rejectedClassifications.push(classification);
                    return error;
                });
        });
        return Promise.all(promises)
            .then(function(values) {
                console.log(values)
            });
    }